Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: [DHIS2-18325] Show orgunit selector in the new event form #3879

Open
wants to merge 27 commits into
base: master
Choose a base branch
from

Conversation

henrikmv
Copy link
Contributor

@henrikmv henrikmv commented Nov 11, 2024

DHIS2-18325

  • Implemented org. unit selector in Report tab
  • Made org. unit field required and added validation in form
  • Blocked the org. unit field in context selector when new event page is open by setting the new isReadOnly prop to true.
  • Data entry form can now be opened without org. unit set in context selector
  • Auto select org. unit in new event form when org. unit is selected in the context selector
  • Removes the cypress for clearing the org. unit from the new event page because this action is now blocked

@henrikmv henrikmv marked this pull request as ready for review December 10, 2024 16:58
@henrikmv henrikmv requested a review from a team as a code owner December 10, 2024 16:58
@henrikmv henrikmv added testing and removed testing labels Dec 10, 2024
Copy link

github-actions bot commented Dec 11, 2024

@henrikmv henrikmv marked this pull request as draft December 11, 2024 11:48
@henrikmv henrikmv removed the testing label Dec 11, 2024
@henrikmv henrikmv added testing and removed testing labels Dec 12, 2024
@henrikmv henrikmv marked this pull request as ready for review December 12, 2024 10:32
@henrikmv henrikmv added testing and removed testing labels Dec 14, 2024
@henrikmv henrikmv added testing and removed testing labels Dec 23, 2024
const validatorContainers = [
{
validator: validateOrgUnit,
message: i18n.t('Please provide an valid organisation unit'),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use errorMessage instead of message, this way we sync with the work done in DHIS2-18325?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, that is a great idea!

Comment on lines +10 to +14
{orgUnitName
? i18n.t('Saving to {{stageName}} for {{programName}} in {{orgUnitName}}',
{ orgUnitName, stageName, programName, interpolation: { escapeValue: false } })
: i18n.t('Saving to {{stageName}} for {{programName}}',
{ stageName, programName, interpolation: { escapeValue: false } })}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I go to the new event page without having an org unit in ScopeSelector and select one in the form, the text at the bottom of the form does not reflect the selected orgUnit. Can you take a look at it?

Screen.Recording.2025-01-09.at.09.43.11.mov

Thanks!

@@ -17,6 +17,9 @@ const convertToClientCoordinates = ({ coordinates, type }: { coordinates: any[],

const getCenterPoint = (InnerComponent: ComponentType<any>) => (props: Object) => {
const { orgUnit, ...passOnProps } = props;
if (!orgUnit || !orgUnit.id) {
return <InnerComponent {...passOnProps} center={DEFAULT_CENTER} onOpenMap={() => {}} />;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the center where the map opens change when a different orgUnit is selected?

Screen.Recording.2025-01-09.at.10.16.25.mov

@@ -96,9 +96,12 @@ export const useLifecycle = ({
]);

const rulesReady =
(!orgUnit) ||
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't the orgUnit mandatory for the rules engine? Should we use && instead of ||?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants